Add a new setting for configuring hostname #1664

zmrow · 2021-07-21T22:19:28Z

Issue number:
Fixes #1608

Description of changes:
Reading through this PR by commit will probably be easiest.

IMPORTANT NOTE
As mentioned in #1608, kubelet uses the hostname when registering itself and for certificate/authorization purposes. Given that fact, changing the hostname after the host's initial boot (and by extension, kubelet's initial registration) may cause issues with the kubelet. We don't currently have a method in the API server for "write once, read many" situations but considered the setting useful enough to move forward. #1663 tracks thoughts on the "write once, read many" API server functionality.

Previously, hostname was written directly to disk by netdog called by wicked early in boot. At the point in the boot sequence when wicked calls netdog, the API server isn't up yet. Rather than rework the boot sequence (which would be tricky as this point in boot) and add the ability to netdog to talk to the API, we opted to add a subcommand to netdog as a settings generator for the new settings.network.hostname setting. The setting is used in a template file, which is an env file for a new systemd unit that calls netdog set-hostname. This command is what will write the hostname to disk at /proc/sys/kernel/hostname.

This means that at boot time, if a user has the hostname setting in their user data, we will use it, otherwise we will set the hostname using largely the same logic as was in the program previously. (The only change is that if the DNS reverse lookup fails, we use the IP in the format "ip-x-x-x" as the hostname).

netdog was reworked to use argh for argument parsing since our custom logic was starting to get unwieldy with the addition of more subcommands with their own arguments.

Testing done:

/etc/resolv.conf is populated in all cases
Migrations work correctly and the setting and its metadata are correctly populated on upgrade, and nonexistent on downgrade
I tested that you can set this setting via apiclient at runtime. kubelet logs some errors. I can't speak to what will happen to the kubelet or if it will continue to function or for how long.

No settings.network.hostname in user data:

Boot aws-k8s-1.19: it starts, gets a correct hostname, and connects to the cluster just fine and runs a pod
Boot vmware-k8s-1.20: it starts, gets the string ip-x-x-x-x as its hostname, connects to the cluster, runs a pod

settings.network.hostname="br-test-1" in user data:

Boot aws-k8s-1.10: it boots, gets the hostname from user data, but (correctly) doesn't ever become ready in the cluster.
Boot vmware-k8s-1.20: it boots, gets hostname from user data, and shows up neatly in the cluster, runs a pod.

> kubectl get nodes
NAME                STATUS   ROLES                  AGE     VERSION
br-test-1           Ready    <none>                 6h19m   v1.20.8
mrowicki-qs-xx6l2   Ready    control-plane,master   6h41m   v1.20.8

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

sources/api/netdog/src/main.rs

bcressey · 2021-07-25T22:24:18Z

sources/api/netdog/src/main.rs

+fn generate_hostname() -> Result<()> {
+    let ip_string =
+        fs::read_to_string(CURRENT_IP).context(error::CurrentIpReadFailed { path: CURRENT_IP })?;
+    let ip = IpAddr::from_str(&ip_string).context(error::IpFromString { ip: &ip_string })?;


It'd be good to add this deserialization check to node_ip() also.

I'll open a separate PR for this.

sources/api/netdog/src/main.rs

sources/models/src/lib.rs

README.md

sources/api/netdog/README.md

sources/api/netdog/src/main.rs

This removes the custom argument parsing logic in favor of using `argh`. `argh` makes it simpler to add additional subcommands that may or may not have their own arguments.

This adds a new subcommand `generate-hostname` that is meant to be used as a settings generator. It will attempt to read the current IP from file and resolve it via DNS reverse lookup. If the lookup is successful it is used, otherwise the IP is used with dots replaced by dashes to avoid any confusion from other software that may attempt to read it, e.g "1.2.3.4" will end up being hostname "ip-1-2-3-4".

This adds a new subcommand `set-hostname` that sets the hostname for the system by writing it to `/proc/sys/kernel/hostname`

zmrow · 2021-07-28T21:10:19Z

^ Addresses @bcressey 's comments!

zmrow · 2021-07-28T21:11:08Z

^ Rebases on develop

sources/api/netdog/src/main.rs

sources/models/src/modeled_types/shared.rs

etungsten

LGTM. ~~If we know changing the hostname during runtime is harmful for k8s variants (might be variant specific). We should probably think of a way to disallow that.~~

edit: sorry just remembered it's already covered in an follow up issue

This commit adds the ability to configure the hostname of the system via a new setting `settings.network.hostname`. If the setting isn't populated, at boot time we use the settings generator `netdog generate-hostname` to populate it. The generator attempts to resolve the current IP via reverse lookup, and if unsuccessful uses the IP to create a hostname in the format "ip-x-x-x-x". A new service `set-hostname` runs after `settings-applier` and writes the hostname to `/proc/sys/kernel/hostname` via `netdog set-hostname $HOSTNAME`. The service unit file uses an env file that is a template using the setting `settings.network.hostname` to populate the $HOSTNAME variable.

This adds the migrations needed for `settings.network.hostname`, one for the setting, its service and configuration file, and one for the setting generator metadata.

etungsten · 2021-07-30T00:58:21Z

Sorry for pointing this out after the fact. The next release version should be v1.2.0 so the migrations path and Release.toml needs to be updated from v1.1.5 to v1.2.0.

diranged · 2021-08-06T18:14:18Z

I'm a bit confused - I thought that EKS forced us to always use the ip-xx-xx-xx-xx pattern, is that not true? Hostnames need to be unique too I believe, so is this something that is passed through a templating engine?

zmrow · 2021-08-06T20:58:00Z

Hi @diranged - as mentioned in the setting documentation in the README, most users shouldn't need to touch this setting as the defaults make sure that the hostname is set the same as it always has been.

In VMware, this setting can be useful to set the hostname the same as the VM name so I expect it will get the most use there.

If you're primarily running in EC2 - you shouldn't ever need to touch this setting. :)

zmrow requested review from bcressey, tjkirch and etungsten July 21, 2021 22:19

zmrow self-assigned this Jul 21, 2021

bcressey reviewed Jul 25, 2021

View reviewed changes

zmrow force-pushed the hostname-setting branch from fcd6b23 to 9393915 Compare July 28, 2021 21:06

zmrow added 3 commits July 28, 2021 21:09

netdog: Switch to argh for argument parsing

e76f8ba

This removes the custom argument parsing logic in favor of using `argh`. `argh` makes it simpler to add additional subcommands that may or may not have their own arguments.

netdog: Add set-hostname subcommand

f6c9943

This adds a new subcommand `set-hostname` that sets the hostname for the system by writing it to `/proc/sys/kernel/hostname`

zmrow force-pushed the hostname-setting branch from 9393915 to f4861b0 Compare July 28, 2021 21:10

bcressey approved these changes Jul 29, 2021

View reviewed changes

sources/api/netdog/src/main.rs Outdated Show resolved Hide resolved

sources/models/src/modeled_types/shared.rs Outdated Show resolved Hide resolved

etungsten approved these changes Jul 29, 2021

View reviewed changes

zmrow added 3 commits July 29, 2021 20:57

Add migrations for settings.network.hostname

9719b40

This adds the migrations needed for `settings.network.hostname`, one for the setting, its service and configuration file, and one for the setting generator metadata.

README.md: Add hostname setting documentation

bda731a

zmrow force-pushed the hostname-setting branch from f4861b0 to bda731a Compare July 29, 2021 21:11

etungsten approved these changes Jul 29, 2021

View reviewed changes

bcressey approved these changes Jul 29, 2021

View reviewed changes

zmrow merged commit ea4cf0a into bottlerocket-os:develop Jul 29, 2021

zmrow deleted the hostname-setting branch July 29, 2021 22:06

zmrow mentioned this pull request Jul 30, 2021

netdog: Add IP validation to node_ip() #1680

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a new setting for configuring hostname #1664

Add a new setting for configuring hostname #1664

zmrow commented Jul 21, 2021 •

edited

Loading

bcressey Jul 25, 2021

zmrow Jul 28, 2021

zmrow commented Jul 28, 2021

zmrow commented Jul 28, 2021

etungsten left a comment •

edited

Loading

etungsten commented Jul 30, 2021

diranged commented Aug 6, 2021

zmrow commented Aug 6, 2021

Add a new setting for configuring hostname #1664

Add a new setting for configuring hostname #1664

Conversation

zmrow commented Jul 21, 2021 • edited Loading

bcressey Jul 25, 2021

Choose a reason for hiding this comment

zmrow Jul 28, 2021

Choose a reason for hiding this comment

zmrow commented Jul 28, 2021

zmrow commented Jul 28, 2021

etungsten left a comment • edited Loading

Choose a reason for hiding this comment

etungsten commented Jul 30, 2021

diranged commented Aug 6, 2021

zmrow commented Aug 6, 2021

zmrow commented Jul 21, 2021 •

edited

Loading

etungsten left a comment •

edited

Loading